fix(places): correct the shipped default station id (#641) - #642
Merged
Conversation
DEFAULT_CONFIG's [[place]] named "Schöneweide" shipped station id 900180001, which is a different, real, nearby station on the live BVG API: "S Köpenick/Parrisiusstr. (Berlin)", served only by S3 — none of which passes through Köpenick. The fetch always succeeded and always filtered to zero rows, rendering as the plain "no matching S-Bahn departures right now" text rather than an error, so the widget looked like a quiet evening instead of telling anyone it was misconfigured. The correct id, verified against the live API, is 900192001 = "S Schöneweide Bhf (Berlin)" — the station the place is actually named after. A comment naming the station in words now sits next to the id so the two can't silently drift apart again. Also reconsidered the lines/directions defaults: a populated filter that's wrong (as here) fails invisibly, while an absent filter shows everything through the station and fails visibly/usefully. Shipped with both axes empty by default, keeping the previous example values as commented-out template lines with a note on what they do. Test fallout: the three places.rs tests that pinned the literal "900180001" (self-consistently, with nothing to check it against) are updated, and a new test ties the default station id to the place name it must match, with the real-world station names spelled out in its failure message. This only changes what write_default_config writes for NEW installs (it only ever writes when places.toml is absent) — existing configs are untouched and need a manual edit. Closes #641
This was referenced Jul 30, 2026
Closed
Merged
vibechoom
added a commit
that referenced
this pull request
Jul 31, 2026
…urst falsified (#672) Closes #660 Bring docs/live-verify.md current across the ten PRs that merged after the last refresh (#634): #637, #639, #644, #645, #662, #663, #664, #666, #668, plus #642 (coverage bump only, per the issue's own conclusion — its default station-id fix needs no live-verify caveat since write_default_config only touches an absent places.toml). Corrected the #630 entry: it told a verifier to expect modal::close_all tearing down with window.close(); #637 changed that call to destroy() after #630 had already merged, so the entry named a call the code no longer makes (exactly the issue's finding). Also added a new #645 entry to the Network panel section for the networkd startup-refresh retry, since that path used to latch "no link manager has answered yet" forever on a transient first-refresh failure (curable only by a shell restart) and now self-heals — swept the rest of the doc for the same class of falsified expectation (inert-until-restart) and found nothing else affected; the wifi-side #609/#634 entries describe a different code path and are unaffected. New entries added: - #637/#639 — the destroy()-not-close() sweep across the five trollshell overlays and the three remaining hytte-ui sites (bar.rs, popup.rs), including #637's WeakRef/toplevels()-count kanshi hot-plug gesture. - #644/#663 — the RefCell-borrow-across-GTK-call sweep (31 sites total across overlays/modal.rs and the rest of the UI, incl. an 8-panel-wide reactive_list.rs helper and the control-center). Both documented with the same "nothing changes" honest pass condition #630 already used. - #664 — mpris Auto chip fix + bare-string artist widening. Needs two live MPRIS players and a shell restart for the new CSS class, both called out. - #666 — new top-level "Claude bridge" section for the keyless loopback shim, nobody has run this service live yet. Carries the PR's flagged riskiest assumption (hive-claude's SessionNotFound classification) with its journal tell. - #668 — new top-level "D-Bus name ownership" section. Documents that recovery going from ~250ms to up to 5 minutes after a squatter exits is the expected cost of the fix, not a regression to file. Left alone: #642 (no doc change needed beyond the coverage line, matching the issue's own recommendation after checking places.toml's write-once behavior); #637's abort_subscription/prompt.rs exclusions and #644/#663's "deliberately not touched" sites (no live-visible behavior, nothing to verify). Gates: markdown-only diff (git diff --stat: 1 file, docs/live-verify.md). nix fmt run twice (1 file reformatted first pass — a prose reflow — 0 changed second pass). Clippy/tests deliberately skipped: nix/package.nix's crane filter excludes markdown, so the packaged derivation is unaffected and a nix build would burn a full workspace recompile for zero signal (the same crane-filter trap #662 already documented). nix build was not run, per instructions. Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #641
The bug, confirmed against the live BVG API
crates/hytte-services/src/places.rs'sDEFAULT_CONFIGshipped a[[place]]named"Schöneweide"withstation = "900180001". Resolving both ids against the live API:900180001→ "S Köpenick/Parrisiusstr. (Berlin)" — a real, different, nearby station. The only S-Bahn line calling there is S3.900192001→ "S Schöneweide Bhf (Berlin)" — the station the place is actually named after.So the default fetched Köpenick and then filtered for
S8/S85/S9, none of which ever call there — a structural, permanent zero-match. The fetch succeeds and the JSON decodes cleanly, so this renders as the plain"no matching S-Bahn departures right now"text (hytte-plugin-departures/src/main.rs'sOk/Staleempty-items arm), not an error — it looks like a quiet evening rather than "I'm misconfigured". That's the entire bug per the issue's triage comment.Fix:
station = "900192001", with a comment naming the station in words right next to the id so the opaque numeric id and the human place name can't silently drift apart again.lines/directionsdefaults — decisionI went with Annika's lean from the issue: ship the default with both axes empty/commented out, not populated.
Reasoning: a populated filter that's wrong (as here) fails invisibly — the widget just shows nothing, indistinguishable from an actually quiet station, exactly how #641 shipped for however long. An empty filter (per
feed.rs:94-97, empty = allow all) fails visibly and usefully instead — a new user sees every suburban line through the station immediately, confirms the widget works, and only then opts into narrowing it. The previous["S8", "S85", "S9"]/["Spandau", "Birkenwerder", "Hohen Neuendorf", "Waidmannslust"]values are kept as commented-out template lines with a note on what each axis does, so the shape is still discoverable.Tests
default_config_parses(was pinning the wrongstationliteral, and the oldlinesvalue) now expects900192001and emptylines/directions.place_transition_silent_on_first_resolution/place_transition_fires_on_genuine_name_changeused"900180001"as an arbitrary fixture id for an unrelated"Home"test place (nothing to do with Schöneweide) — swapped for an obviously-synthetic id (900000001) so no test in the file keeps the buggy literal lying around to confuse a future grep.default_station_id_matches_its_place_name: ties the default place'sname("Schöneweide") to itsstationid (900192001), with an assertion failure message that spells out the real-world station names (900192001= S Schöneweide Bhf,900180001= S Köpenick/Parrisiusstr., Departures broken #641) so a future editor who changes this literal sees exactly what they're changing and why the old value was a trap. A bareassert_eq!against a self-chosen literal — which is exactly what let this ship — can't catch this class of bug; pinning the id/name relationship can.Falsification (per the issue's ask)
Reverted
stationback to"900180001"(only that literal, inDEFAULT_CONFIG) and rerancargo test -p hytte-services places::tests:Then restored the fix and confirmed the file is byte-identical to before the revert (
diffclean), and reran the full suite green:cargo test -p hytte-services→ 672 passed; 0 failed.Gates run
unset RUSTC_WRAPPER && cargo clippy -p hytte-services --all-targets --features system-tests -- -D warnings— clean.cargo test -p hytte-services— 672 passed, 0 failed.nix fmttwice — 0 files changed both times.What this does NOT fix
Existing installs are unaffected.
write_default_config(places.rs:329) only writesplaces.tomlwhen the file is absent, so this corrects the default for new installs only. If you already have a~/.config/trollshell/places.tomlwith the old900180001/S8,S85,S9values (as Annika does), this PR does nothing for it — you need to hand-edit that file yourself (changestationto900192001, and/or drop thelines/directionslines to see everything). This repo intentionally was not touched outsidecrates/hytte-services/src/places.rs; nothing undercrates/hytte-plugin-departures/changed (its fetch/filter logic was already correct — this was purely a wrong-constant bug in the shipped config), and no config files outside the repo were touched.